Skip to content

Conversation

Griffon-Weglot
Copy link

@Griffon-Weglot Griffon-Weglot commented Sep 12, 2025

Q A
Bug fix? no
New feature? yes
Docs? no
Issues #17
License MIT
sample2

@Griffon-Weglot Griffon-Weglot changed the title feat(profiler): trace and display AgentInterface calls [Profiler] trace and display AgentInterface calls Sep 12, 2025
@welcoMattic welcoMattic changed the title [Profiler] trace and display AgentInterface calls [AiBundle] Trace and display AgentInterface calls in Profiler Sep 12, 2025
@welcoMattic welcoMattic added AI Bundle Issues & PRs about the AI integration bundle Feature New feature Hackathon 2025 This issue or pull request was part of the Symfony AI Hackathon 2025 labels Sep 12, 2025
@carsonbot carsonbot changed the title [AiBundle] Trace and display AgentInterface calls in Profiler [AI Bundle][AiBundle] Trace and display AgentInterface calls in Profiler Sep 12, 2025
@OskarStark
Copy link
Contributor

Can you please add a screenshot to the PR header?

@Griffon-Weglot Griffon-Weglot force-pushed the main branch 2 times, most recently from 899f7db to 95d76eb Compare September 13, 2025 07:37

public function collect(Request $request, Response $response, ?\Throwable $exception = null): void
{
$this->lateCollect();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change?

Copy link
Author

@Griffon-Weglot Griffon-Weglot Sep 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seemed redundant to call late collect since it will be called on kernel.terminate so i've removed it, also it's going againts the idea of implementing LateDataCollectorInterface to me, do you think it's relevant ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was introduced in #173 with the reason - does this change break the profiler on streaming?

Copy link
Member

@chr-hertel chr-hertel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this @Griffon-Weglot!

One thing that would help is to move the input/output html conversion that is used on the platform calls to some macro, that we can use it on the agent and on the platform. so we wouldn't need to just dump the those.

@Griffon-Weglot
Copy link
Author

Griffon-Weglot commented Sep 15, 2025

One thing that would help is to move the input/output html conversion that is used on the platform calls to some macro, that we can use it on the agent and on the platform. so we wouldn't need to just dump the those.

@chr-hertel to simplify the display i've used profiler_dump since it's doing that very well

@OskarStark OskarStark changed the title [AI Bundle][AiBundle] Trace and display AgentInterface calls in Profiler [AI Bundle] Trace and display AgentInterface calls in Profiler Oct 3, 2025
Comment on lines -106 to -117
$calls = $platform->calls;
foreach ($calls as $key => $call) {
$result = $call['result']->await();

if (isset($platform->resultCache[$result])) {
$call['result'] = $platform->resultCache[$result];
} else {
$call['result'] = $result->getContent();
}

$calls[$key] = $call;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is that necessary? also see #173 for reasoning

@chr-hertel
Copy link
Member

please bring up the metrics again to the top:
image

@Griffon-Weglot
Copy link
Author

please bring up the metrics again to the top

Sure ! Thanks for your feedbacks i'll make those changes asap

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AI Bundle Issues & PRs about the AI integration bundle Feature New feature Hackathon 2025 This issue or pull request was part of the Symfony AI Hackathon 2025 Status: Needs Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants